Code Review

This pull request fixes a half-pixel offset issue when importing images without a specific mouse position by using DAffine2::IDENTITY to default to the document origin. The change is applied to both PasteImage and PasteSvg message handlers. The fix appears correct. My only feedback is regarding code duplication between these two handlers, which could be refactored into a helper function to improve maintainability.

2 issues found across 1 file

Confidence score: 3/5

• There is a concrete regression risk in editor/src/messages/portfolio/document/document_message_handler.rs: both PasteImage and PasteSvg now fall back mouse: None to document origin, which can place pasted content far from the user’s current view.
• This is medium severity but high confidence (6/10, 8/10), so the PR likely remains workable yet carries noticeable user-facing paste behavior risk in existing documents.
• Pay close attention to editor/src/messages/portfolio/document/document_message_handler.rs - verify mouse: None paste placement logic so clipboard pastes stay visible in non-origin views.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/portfolio/document/document_message_handler.rs">

<violation number="1" location="editor/src/messages/portfolio/document/document_message_handler.rs:667">
P2: `PasteImage` now treats `mouse: None` as document origin for all flows, which regresses clipboard paste placement in existing documents.</violation>

<violation number="2" location="editor/src/messages/portfolio/document/document_message_handler.rs:721">
P2: `PasteSvg` applies the same `mouse: None -> origin` fallback, causing clipboard SVG paste to appear off-screen in non-origin views.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}

Code Review

This pull request effectively addresses the half-pixel offset issue for imported images and SVGs by rounding translation values. The changes also introduce significant improvements to the import process, such as respecting the SVG viewBox for artboard creation and refactoring the transform handling for SVG imports to be more robust and clear.

I've identified a couple of areas for improvement. Firstly, the rounding of translation is not applied to pasted images, which is inconsistent with the fix for SVGs and could leave the original issue partially unresolved. Secondly, there's a magic number used for node positioning in the node graph that could be replaced with a constant for better maintainability.

Overall, this is a great set of changes that improves both correctness and code quality.

4 issues found across 6 files

Confidence score: 3/5

• There is a concrete regression risk in editor/src/messages/portfolio/document/document_message_handler.rs: translation rounding is applied to SVG paths but not image transforms, so pasted/dropped images can still land on subpixel positions and appear misaligned relative to imported paths.
• editor/src/messages/portfolio/document/document_message.rs and editor/src/messages/portfolio/portfolio_message_handler.rs both round away fractional viewBox data (IVec2 conversion and origin/size rounding), which can shift artboards and clip edge content during SVG import.
• Given multiple medium-to-high severity import/placement issues (6–7/10) with strong confidence, this is some user-facing layout risk rather than a pure housekeeping change.
• Pay close attention to editor/src/messages/portfolio/document/document_message_handler.rs, editor/src/messages/portfolio/document/document_message.rs, and editor/src/messages/portfolio/portfolio_message_handler.rs - rounding behavior may cause subpixel misalignment and fractional-edge clipping on import.

Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.


<file name="editor/src/messages/portfolio/document/document_message.rs">

<violation number="1" location="editor/src/messages/portfolio/document/document_message.rs:234">
P2: Using `IVec2` for `artboard_canvas` loses fractional SVG `viewBox` values, causing subpixel offsets to be rounded away during import.</violation>
</file>

<file name="editor/src/messages/portfolio/document/document_message_handler.rs">

<violation number="1" location="editor/src/messages/portfolio/document/document_message_handler.rs:668">
P1: The image transform's translation is not rounded, but the SVG path rounds it (`placement_transform.translation = placement_transform.translation.round()`). This inconsistency means pasted/dropped images can still land at sub-pixel positions, causing the half-pixel offset this PR is fixing. Make `transform` mutable and round its translation after computing it.</violation>

<violation number="2" location="editor/src/messages/portfolio/document/document_message_handler.rs:1400">
P3: Replace the magic number `15` with `LAYER_INDENT_OFFSET + 7` to stay in sync if the indent constant ever changes. `LAYER_INDENT_OFFSET` is already imported.</violation>
</file>

<file name="editor/src/messages/portfolio/portfolio_message_handler.rs">

<violation number="1" location="editor/src/messages/portfolio/portfolio_message_handler.rs:720">
P2: Rounding viewBox origin/size to nearest integer can undersize the imported SVG artboard and clip fractional-edge content.</violation>
</file>

_{Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.}